[fix](regression) Deflake segcompaction agg keys case by shuke987 · Pull Request #65382 · apache/doris

shuke987 · 2026-07-08T11:38:21Z

Proposed changes

Follow up the existing test_segcompaction_agg_keys deflake from #61026.

The current master case already avoids the old golden file by allowing two concrete full-row results for col_0=47. This patch keeps the same intent but removes the hard-coded full-row candidates. The case now checks stable invariants instead:

col_0=47 returns exactly one aggregate row.
The row has the expected 50 columns.
The key is 47.
All REPLACE value columns belong to the known fruit value domain in the ORC input.

This avoids pinning the test to a specific duplicate-key REPLACE winner while still checking that the load and aggregation returned a valid row.

Why

The ORC input contains multiple rows with col_0=47. With AGGREGATE KEY(col_0) and REPLACE value columns, the case should not require one specific source row to win unless the data/model makes that winner deterministic.

Validation

Remote branch-4.1 regression environment:

ssh root@172.20.56.184
cd /deploy/teamcity_agent13/work/a0477ef061788419/case_output/doris
export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64
export PATH=$JAVA_HOME/bin:$PATH
./run-regression-test.sh --run -d segcompaction_p2 -s test_segcompaction_agg_keys

Result:

Test 1 suites, failed 0 suites, fatal 0 scripts, skipped 0 scripts

hello-stephen · 2026-07-08T11:38:26Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

shuke987 · 2026-07-08T11:38:32Z

run buildall

shuke987 · 2026-07-08T11:38:39Z

/review

github-actions

Review result: Request changes.

Critical checkpoints:

Goal and test coverage: the PR intends to deflake test_segcompaction_agg_keys, but the new assertion no longer proves the full aggregate-key REPLACE winner row; see the inline comment.
Scope: the change is focused to one regression test file. No production code, config, session propagation, persistence, or compatibility change is present.
Parallel paths: sibling segcompaction outputs expose the complete duplicate source rows, so the test can preserve row-level coverage without pinning one deterministic winner.
Concurrency, lifecycle, data writes, and observability: not applicable to this test-only assertion change.
Validation: static review only in this runner. .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc are absent, so I did not run the regression suite locally.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions:

optimizer-rewrite: no new valuable findings; the optimizer/rewrite concern was dismissed as not applicable to this one-file test assertion change.
tests-session-config: TSC-001 was accepted as MAIN-001 and submitted inline.
Convergence: round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.

github-actions · 2026-07-08T11:48:30Z

+        assertEquals(50, result[0].size())
+        assertEquals("47", result[0][0].toString())
+
+        def validValues = [


This is too weak for the regression: after this change any row whose 49 value cells are fruit names passes, even if the row is assembled from different duplicate rows or the wrong source columns. The same data set already exposes the 12 complete col_0=47 source rows in test_segcompaction_dup_keys.out, and the aggregate-key REPLACE result should be one complete winner row, not just 49 independently valid vocabulary entries. To keep the deflake while preserving coverage, please compare result[0] against the set of complete allowed source rows instead of checking each cell against validValues.

shuke987 · 2026-07-09T09:55:58Z

/review

shuke987 · 2026-07-09T09:55:59Z

run buildall

github-actions · 2026-07-09T10:01:36Z

Codex automated review failed and did not complete.

Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 11:47 AM.
Workflow run: https://github.com/apache/doris/actions/runs/29009896654

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

shuke987 · 2026-07-09T10:37:08Z

/review

github-actions · 2026-07-09T10:45:55Z

Codex automated review failed and did not complete.

Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at 11:47 AM.
Workflow run: https://github.com/apache/doris/actions/runs/29012196550

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

shuke987 · 2026-07-09T11:56:49Z

/review

github-actions

Automated review summary for PR 65382 at head 08a4c4b.

I reviewed the changed regression test and found no new issue requiring an inline comment. The earlier inline thread about the fruit-domain-only assertion is addressed in the current head: the test now loads the 12 complete col_0=47 rows from test_segcompaction_dup_keys.out and requires the aggregate-key result row to match one complete source row.

Critical checkpoint conclusions:

Goal and tests: the change keeps the deflake goal while preserving row-level REPLACE coverage for the duplicate key case.
Scope: the PR is narrowly scoped to regression-test/suites/segcompaction_p2/test_segcompaction_agg_keys.groovy.
Concurrency, lifecycle, configuration, compatibility, transactions, persistence, and observability: not applicable for this test-only assertion change.
Parallel paths: the duplicate-key fixture used as the source-row oracle matches the same ORC input shape and contains the 12 expected col_0=47 rows.
Regression standards: no generated .out update is needed because this suite uses explicit assertions for the nondeterministic winner row; git diff --check is clean.
Validation: static review plus GitHub PR metadata; I did not run the S3 regression locally in this uninitialized shallow runner checkout.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions: optimizer-rewrite and tests-session-config both found no new valuable candidates. Convergence round 1 ended with both subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and empty inline-comment set.

shuke987 · 2026-07-09T12:06:45Z

run buildall

shuke987 · 2026-07-09T12:37:41Z

/review

github-actions

Automated review summary for PR 65382 at head 08a4c4b.

I reviewed the changed regression test and found no new issue requiring an inline comment. The earlier inline thread about the weak fruit-domain-only assertion is addressed in the current head: the test now loads the 12 complete col_0=47 rows from test_segcompaction_dup_keys.out and requires the aggregate-key result row to match one complete source row.

Critical checkpoint conclusions:

Goal and tests: the change keeps the deflake goal while preserving row-level REPLACE coverage for the duplicate-key case.
Scope: the PR is narrowly scoped to regression-test/suites/segcompaction_p2/test_segcompaction_agg_keys.groovy.
Concurrency, lifecycle, configuration, compatibility, transactions, persistence, and observability: not applicable for this test-only assertion change.
Parallel paths: the duplicate-key fixture used as the source-row oracle matches the same ORC input shape and contains the 12 expected col_0=47 rows.
Regression standards: no generated .out update is needed because this suite uses explicit assertions for the nondeterministic winner row; git diff --check is clean.
Validation: static review plus GitHub PR metadata; I did not run the S3 regression locally in this uninitialized shallow runner checkout.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions: optimizer-rewrite and tests-session-config both found no new valuable candidates in their focused passes. Convergence round 1 ended with both subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and empty inline-comment set after the final main-owned duplicate/resolution update.

[fix](regression) Deflake segcompaction agg keys case

b66744c

github-actions Bot requested changes Jul 8, 2026

View reviewed changes

[fix](regression) Preserve segcompaction source row coverage

08a4c4b

github-actions Bot reviewed Jul 9, 2026

View reviewed changes

shuke987 closed this Jul 10, 2026

Uh oh!

Conversation

shuke987 commented Jul 8, 2026

Proposed changes

Why

Validation

Uh oh!

hello-stephen commented Jul 8, 2026

Uh oh!

shuke987 commented Jul 8, 2026

Uh oh!

shuke987 commented Jul 8, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

shuke987 commented Jul 9, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants